Skip to content

Various Play insight fixes - #1367

Merged
gerhardol merged 12 commits into
jonasoreland:masterfrom
gerhardol:feature/play-insight-fixes
Aug 9, 2026
Merged

Various Play insight fixes#1367
gerhardol merged 12 commits into
jonasoreland:masterfrom
gerhardol:feature/play-insight-fixes

Conversation

@gerhardol

Copy link
Copy Markdown
Collaborator

Various Play insight fixes, mostly fixed with Gemini
I expect a few of them are not really addressing the core issue with activity or db "lost", but they could give better insight at next occurrence.
Could test only a few of them, before and after the fixes, most are not reproducible at all.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a set of defensive checks and parsing adjustments intended to reduce crashes and improve diagnostics around “lost” activity/DB states and Google Play insights, primarily by hardening null handling and numeric parsing.

Changes:

  • Replace several direct Double.parseDouble(...) calls with SafeParse.parseDouble(...) and expand SafeParse.parseDouble behavior.
  • Add guards/logging around potentially-null runtime components (tracker, DB, Activity contexts, dialogs/spinners).
  • Adjust workout step insertion and notification update behavior to be more resilient during runtime.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
app/src/main/org/runnerup/workout/WorkoutBuilder.java Uses SafeParse for autolap parsing; adjusts step insertion logic for countdown/rest steps.
app/src/main/org/runnerup/workout/Step.java Guards s.tracker.resume() against null and logs a warning.
app/src/main/org/runnerup/workout/RepeatStep.java Adds bounds-check in getTime() to avoid out-of-range crashes.
app/src/main/org/runnerup/view/StartFragment.java Avoids requireActivity() crash when requesting permissions from a detached fragment.
app/src/main/org/runnerup/view/ManualActivity.java Uses SafeParse.parseDouble for manual distance entry.
app/src/main/org/runnerup/view/AudioCueSettingsFragment.java Adds scheme-name sanitization and extra null-safety when opening prefs.
app/src/main/org/runnerup/util/SafeParse.java Changes parseDouble behavior (including comma normalization).
app/src/main/org/runnerup/tracker/Tracker.java Adds null checks/logging for DB usage and writer usage.
app/src/main/org/runnerup/notification/ForegroundNotificationDisplayStrategy.java Avoids repeated startForeground by tracking foreground state and using NotificationManager.notify for updates.
app/src/main/org/runnerup/export/SyncManager.java Adds null checks around auth activity start and spinner title updates.
app/src/main/org/runnerup/export/RunKeeperSynchronizer.java Uses SafeParse for autolap preference parsing.
app/src/main/org/runnerup/export/format/RunKeeper.java Uses SafeParse for distance parsing from JSON.
app/src/main/org/runnerup/db/PathSimplifier.java Uses SafeParse for tolerance preference parsing with fallback defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/src/main/org/runnerup/util/SafeParse.java
Comment thread app/src/main/org/runnerup/view/AudioCueSettingsFragment.java Outdated
Comment on lines 327 to 333
String scheme = editText.getText().toString();
if (!scheme.contentEquals("")) {
createNewAudioScheme(scheme);
updateSortOrder(scheme);
switchTo(scheme);
if (scheme.isEmpty()
|| scheme.contains("/")
|| scheme.contains("\\")
|| scheme.contains("..")) {
return;
}
Comment thread app/src/main/org/runnerup/workout/WorkoutBuilder.java
Comment thread app/src/main/org/runnerup/workout/RepeatStep.java
Comment thread app/src/main/org/runnerup/tracker/Tracker.java
gerhardol added a commit to gerhardol/runnerup that referenced this pull request Jul 30, 2026
As speech is asynch, a fragment may be detached when it occurs.
@gerhardol
gerhardol force-pushed the feature/play-insight-fixes branch from 1342b1f to 3d3b31d Compare July 30, 2026 21:07
@gerhardol
gerhardol requested a review from Copilot July 30, 2026 21:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (4)

app/src/main/org/runnerup/view/AudioCueSettingsFragment.java:73

  • settingsName is now sanitized before being used as the scheme identifier, but other code in this fragment still treats the scheme name as the raw DB value (e.g. spinner selection uses adapter.find(settingsName) and DB operations use AUDIO_SCHEMES.NAME). For scheme names containing spaces/special chars, the sanitized settingsName will no longer match DB/adapter values, leading to wrong spinner selection and inability to reliably detect/avoid redundant switchTo() replacements.

This issue also appears on line 231 of the same file.

    settingsName = sanitizeSettingsName(requireArguments().getString("name"));

    if (settingsName != null) {
      PreferenceManager prefMgr = getPreferenceManager();
      prefMgr.setSharedPreferencesName(settingsName + SUFFIX);

app/src/main/org/runnerup/view/AudioCueSettingsFragment.java:233

  • deleteAudioScheme() passes settingsName (sanitized) into deleteAudioSchemeImpl(), but the DB row is stored under the raw scheme name (createNewAudioScheme inserts the unsanitized value). This causes the DB delete to fail for schemes whose name is changed by sanitization (e.g. spaces -> underscores).
                + sanitizeSettingsName(name)
                + SUFFIX
                + ".xml");

app/src/main/org/runnerup/tracker/Tracker.java:606

  • setNextLocationType() now returns early when mDBWriter is null, but it also skips updating mLocationType. That can leave the in-memory location type stale and can affect subsequent state-machine decisions/logging that rely on mLocationType.
    if (mDBWriter == null) {
      android.util.Log.w(
          "Tracker", "setNextLocationType: mDBWriter is null (newType=" + newType + ")");
      return;
    }

app/src/main/org/runnerup/notification/ForegroundNotificationDisplayStrategy.java:43

  • In the non-foreground branch, service.getSystemService(Context.NOTIFICATION_SERVICE) can return null; notificationManager.notify(...) would then throw an NPE. This would make notification updates crash instead of being a no-op/fallback.
    } else {
      android.app.NotificationManager notificationManager =
          (android.app.NotificationManager) service.getSystemService(Context.NOTIFICATION_SERVICE);
      notificationManager.notify(notificationId, notification);
    }

Comment on lines 566 to +570
private void saveActivity(Double manualDistance) {
if (mDB == null) {
android.util.Log.e("Tracker", "saveActivity called but mDB is null");
return;
}
gerhardol added a commit to gerhardol/runnerup that referenced this pull request Jul 31, 2026
As speech is asynch, a fragment may be detached when it occurs.
@gerhardol
gerhardol force-pushed the feature/play-insight-fixes branch from 3d3b31d to a4dfe99 Compare July 31, 2026 20:12
notify as normal in foreground.
If onDestroy gets called before saveActivity(), the
workout cannot be saved. Avoid crash.
allow comm vs point

If no previous default, assume 0 is OK defaults
No dialog to request permissions if the activity is gone
likely no steps for this workout
As speech is asynch, a fragment may be detached when it occurs.
Illegal characters causes exceptions
@gerhardol
gerhardol force-pushed the feature/play-insight-fixes branch from a4dfe99 to 5d261bb Compare August 9, 2026 13:03
@gerhardol
gerhardol force-pushed the feature/play-insight-fixes branch from 5d261bb to 2e6b7ae Compare August 9, 2026 13:16
@gerhardol
gerhardol merged commit a581637 into jonasoreland:master Aug 9, 2026
1 check passed
gerhardol added a commit that referenced this pull request Aug 9, 2026
As speech is asynch, a fragment may be detached when it occurs.
@gerhardol
gerhardol deleted the feature/play-insight-fixes branch August 9, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants